Skip to content

Fix query for room participation#18345

Merged
erikjohnston merged 2 commits into
developfrom
erikj/participation_fix
Apr 16, 2025
Merged

Fix query for room participation#18345
erikjohnston merged 2 commits into
developfrom
erikj/participation_fix

Conversation

@erikjohnston

@erikjohnston erikjohnston commented Apr 16, 2025

Copy link
Copy Markdown
Member

Follow on from #18068

Currently the subquery in UPDATE is pointless, as it will still just update all room_membership rows. Instead, we should look at the current membership event ID (which is easily retrieved from local_current_membership). We also add a AND NOT participant to noop the UPDATE when the participant flag is already set.

cc @H-Shay

@erikjohnston erikjohnston requested a review from a team as a code owner April 16, 2025 11:05
ORDER BY event_stream_ordering DESC
LIMIT 1
WHERE event_id IN (
SELECT event_id FROM local_current_membership

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that right? local_current_membership seems to only have memberships for our users whereas room_memberships also has remote users?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is only for local users though

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erik Sorry for the hassle here - the intention with this statement and the one below was to get/set the participant value just for the user's most recent entry in room_memberships. The original desire was to track both local and non-local participation, but thinking through it a bit I don't think it's a huge problem to limit it to local users (and since this is only called in messages it's not tracking federation participation currently anyway), especially if that will reduce resource consumption.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries. As far as I can tell we only call set_room_participation from EventCreationHandler, which only deals with local users.

The query is easy to convert to also working with remote users by swapping local_current_membership to current_state_events table.

@erikjohnston erikjohnston merged commit c16a981 into develop Apr 16, 2025
@erikjohnston erikjohnston deleted the erikj/participation_fix branch April 16, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants